Skip to content

Few updates#21

Merged
lunny merged 4 commits intolunny:lunny/webauthnfrom
zeripath:lunny/webauthn
Jan 12, 2022
Merged

Few updates#21
lunny merged 4 commits intolunny:lunny/webauthnfrom
zeripath:lunny/webauthn

Conversation

@zeripath
Copy link

@zeripath zeripath commented Jan 9, 2022

  • Use jwt/v4 everywhere and use fork of markbates/goth that uses jwt/v4
  • Use the same webauthnCredential definition in the migration
  • Adjust the comments to make them no longer a direct copy and add exists/name checks + further highlight we need to handle the errors properly.

@lunny
Copy link
Owner

lunny commented Jan 10, 2022

Please rebase.

Signed-off-by: Andrew Thornton <art27@cantab.net>
Signed-off-by: Andrew Thornton <art27@cantab.net>
Signed-off-by: Andrew Thornton <art27@cantab.net>
Signed-off-by: Andrew Thornton <art27@cantab.net>
type WebAuthnCredential struct {
ID int64 `xorm:"pk autoincr"`
Name string `xorm:"unique(s)"`
LowerName string
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, the tag xorm:"unique(s)" should be in the LowerName field.

// Ensure user is in a WebAuthn session.
idSess := ctx.Session.Get("twofaUid")
if idSess == nil {
idSess, ok := ctx.Session.Get("twofaUid").(int64)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It will panic if there is no session named twofaUid.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The use of the x, ok := something.(otherthing) form prevents the NPE if something is a nil interface{}

@6543
Copy link

6543 commented Jan 11, 2022

link go-gitea#17957

@zeripath
Copy link
Author

OK i've fixed the ci and I've fixed a few more bugs with suburl and users.

@lunny lunny merged commit efec4fd into lunny:lunny/webauthn Jan 12, 2022
@lunny
Copy link
Owner

lunny commented Jan 12, 2022

Let's move on.

@zeripath zeripath deleted the lunny/webauthn branch January 12, 2022 08:00
@zeripath
Copy link
Author

Somehow all of the commits to this branch did not make it across. This is weird.

@zeripath
Copy link
Author

I'm just gonna push these up directly to lunny/webauthn

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants